Skip to content
This repository was archived by the owner on Mar 24, 2025. It is now read-only.

eth-wallet support #547

Open
wants to merge 8 commits into
base: beta
Choose a base branch
from
Open

eth-wallet support #547

wants to merge 8 commits into from

Conversation

@rubenmarcus
Copy link
Contributor

@rubenmarcus rubenmarcus commented Oct 8, 2024 •
edited by mintbase-codium-pr-agent bot
Loading

PR Type

enhancement, dependencies

Description

  • Added support for Ethereum wallets by integrating setupEthereumWallets into the wallet setup process.
  • Configured wagmiConfig and web3Modal to manage Ethereum wallet connections.
  • Updated package.json to include new dependencies required for Ethereum wallet support.
  • Introduced evmWalletChains configuration for testnet and mainnet environments.

Changes walkthrough 📝

Relevant files
Enhancement
bitte-wallet.ts
Add Ethereum wallet support and configuration                       

packages/react/src/wallet/bitte-wallet.ts

  • Added support for Ethereum wallets using setupEthereumWallets.
  • Imported wagmiConfig and web3Modal for wallet configuration.
  • Introduced alwaysOnboardDuringSignIn variable.
    		• +5/-1     
    wallet.ts
    Integrate Ethereum wallets into wallet setup                         

    packages/react/src/wallet/wallet.ts

  • Added support for Ethereum wallets using setupEthereumWallets.
  • Imported wagmiConfig and web3Modal for wallet configuration.
  • Introduced alwaysOnboardDuringSignIn variable.
    		• +5/-2     
    web3-modal.ts
    Configure Web3 modal for Ethereum wallet integration         

    packages/react/src/wallet/web3-modal.ts

  • Created new configuration for Ethereum wallet integration.
  • Defined evmWalletChains for testnet and mainnet.
  • Configured wagmiConfig and web3Modal for wallet connection.
    		• +77/-0   
    Dependencies
    package.json
    Update dependencies for Ethereum wallet integration           

    packages/react/package.json

  • Updated dependencies for wallet selector packages.
  • Added new dependencies for Ethereum wallet support.
    		• +15/-7   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @mintbase-codium-pr-agent
    Copy link

    mintbase-codium-pr-agent bot commented Oct 8, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Dependency Injection
    The setupEthereumWallets function is directly using wagmiConfig and web3Modal from imports without any dependency injection mechanism. This could lead to difficulties in unit testing and make the code less modular. Consider using dependency injection to improve testability and modularity.

    Hardcoded Values
    The file contains hardcoded values for reownProjectId and assumes onMainnet is true. This could lead to issues when moving between different environments (development, staging, production). It's recommended to manage these configurations through environment variables or a configuration management system.

    @mintbase-codium-pr-agent
    Copy link

    mintbase-codium-pr-agent bot commented Oct 8, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Use environment variables for sensitive configuration

    The reownProjectId is hardcoded, which might not be suitable for different
    environments or might expose sensitive information. Consider fetching this value
    from environment variables or a configuration file.

    packages/react/src/wallet/web3-modal.ts [28]

    -const reownProjectId = 'b6facfc0dfb00812382fe1b7bcc07069';
+const reownProjectId = process.env.REOWN_PROJECT_ID;
    Suggestion importance[1-10]: 9

    Why: Using environment variables for the reownProjectId enhances security by preventing exposure of sensitive information, which is a crucial improvement.

    		9
    Possible issue
    Add error handling for setting up Ethereum wallets

    It is recommended to handle potential exceptions or errors that might occur during
    the setup of Ethereum wallets. You can wrap the setupEthereumWallets call in a
    try-catch block to manage exceptions gracefully.

    packages/react/src/wallet/bitte-wallet.ts [44]

    -setupEthereumWallets({ wagmiConfig, web3Modal, alwaysOnboardDuringSignIn })
+try {
+  setupEthereumWallets({ wagmiConfig, web3Modal, alwaysOnboardDuringSignIn })
+} catch (error) {
+  console.error("Failed to setup Ethereum wallets:", error);
+}
    Suggestion importance[1-10]: 8

    Why: Adding error handling for the setupEthereumWallets function is a significant improvement as it enhances the robustness of the code by managing potential exceptions.

    		8
    Enhancement
    Provide default icons in metadata configuration

    The icons array in the metadata object is empty, which might lead to missing icons
    in UI components. Provide default icons or fetch them dynamically if necessary.

    packages/react/src/wallet/web3-modal.ts [56]

    -icons: [],
+icons: ['default-icon.png'],
    Suggestion importance[1-10]: 6

    Why: Adding default icons to the metadata configuration can improve the user interface by ensuring icons are displayed, but it is a minor enhancement.

    		6
    Maintainability
    Improve variable naming for clarity

    Consider using a more descriptive variable name instead of alwaysOnboardDuringSignIn
    to clarify its purpose. A name like forceOnboardingDuringSignIn might better convey
    the intent that onboarding is mandatory during the sign-in process.

    packages/react/src/wallet/bitte-wallet.ts [39]

    -const alwaysOnboardDuringSignIn = true;
+const forceOnboardingDuringSignIn = true;
    Suggestion importance[1-10]: 5

    Why: The suggestion to rename the variable to forceOnboardingDuringSignIn improves clarity and better conveys the intent, but it is a minor improvement in terms of maintainability.

    		5

    Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

    Reviewers

    @GAllen98 GAllen98 Awaiting requested review from GAllen98 GAllen98 is a code owner

    @bh2smith bh2smith Awaiting requested review from bh2smith bh2smith is a code owner

    @marcelokunze marcelokunze Awaiting requested review from marcelokunze marcelokunze is a code owner

    @tifrel tifrel Awaiting requested review from tifrel tifrel is a code owner

    @SurgeCode SurgeCode Awaiting requested review from SurgeCode SurgeCode is a code owner

    @sainthiago sainthiago Awaiting requested review from sainthiago sainthiago is a code owner

    @microchipgnu microchipgnu Awaiting requested review from microchipgnu microchipgnu is a code owner

    @nategeier nategeier Awaiting requested review from nategeier nategeier is a code owner

    @Jxnis Jxnis Awaiting requested review from Jxnis Jxnis is a code owner

    At least 1 approving review is required to merge this pull request.

    Assignees

    No one assigned

    Labels

    dependencies enhancement New feature or request Review effort [1-5]: 3

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @rubenmarcus